Aggregating bounded min/max values has to not ignore null values - #8169
Aggregating bounded min/max values has to not ignore null values#8169robert3005 wants to merge 1 commit into
Conversation
gatesn
left a comment
gatesn
left a comment
There was a problem hiding this comment.
This seems like a very odd behaviour for truncated min/max to me, just leaving a request changes so I can read thoroughly on Monday.
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | decompress_rd[f64, (10000, 0.01)] |
108.7 µs | 139.2 µs | -21.94% |
| ❌ | Simulation | decompress_rd[f64, (10000, 0.1)] |
109 µs | 139.6 µs | -21.9% |
| ❌ | Simulation | decompress_rd[f64, (10000, 0.0)] |
108.7 µs | 139.2 µs | -21.88% |
| ❌ | Simulation | decompress_rd[f32, (100000, 0.0)] |
496 µs | 583.9 µs | -15.06% |
| ❌ | Simulation | decompress_rd[f32, (10000, 0.1)] |
78.1 µs | 91.3 µs | -14.48% |
| ❌ | Simulation | decompress_rd[f32, (10000, 0.01)] |
78.1 µs | 91.1 µs | -14.25% |
| ❌ | Simulation | decompress_rd[f32, (10000, 0.0)] |
78.5 µs | 91.3 µs | -13.96% |
| ⚡ | Simulation | chunked_varbinview_opt_canonical_into[(1000, 10)] |
206.8 µs | 170.2 µs | +21.45% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
215.3 ns | 186.1 ns | +15.67% |
| ⚡ | Simulation | chunked_varbinview_into_canonical[(100, 100)] |
307.1 µs | 272.5 µs | +12.71% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[1024] |
275.6 ns | 246.4 ns | +11.84% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing rk/fuzzer (fc8953a) with develop (35e4d72)
|
This comes from the fact that bounded min/max is not transitive if you exclude nulls. The problem we run into is when producing a bounded max returns null because the bound exceeds the limits we have defined which then is not included in the min/max of the file which then can lead to skipped values. |
|
talked to @joseph-isaacs. |
|
what is the counterexample? |
|
take |
|
But |
|
yes but I don't follow |
|
Okay we need to store a flag to specify |
|
This PR has been marked as stale because it has been open for 14 days with no activity. Please comment or remove the stale label if you wish to keep it active, otherwise it will be closed in 7 days |
Signed-off-by: Robert Kruszewski <github@robertk.io>
|
done in #8515 |
3 participants
Propagating bounded min/max is not the same as propagating nulls. In the case of
bounded min/max we have to conclude that null value could have come from trying
to produce the bounded value which means that there's no valid bounded value while there's still valid values in the array
handle #8166